Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Unified the naming of RLP encoded parameters #528

Merged
merged 5 commits into from
Dec 10, 2018
Merged

Unified the naming of RLP encoded parameters #528

merged 5 commits into from
Dec 10, 2018

Conversation

schemar
Copy link
Contributor

@schemar schemar commented Dec 6, 2018

The name of an RLP encoded parameter now always starts with 'rlp' (or
_rlp in case of a function argument) followed by the name, without the
word Encoded in between.

Examples:

  • rlpAccount
  • rlpParentNodes

BAD example: rlpEncodedParentNodes

Fixes #462

@schemar
Unified the naming of RLP encoded parameters
c7b6193 
The name of an RLP encoded parameter now always starts with 'rlp' (or
`_rlp` in case of a function argument) followed by the name, without the
word `Encoded` in between.

Examples:

* `rlpAccount`
* `rlpParentNodes`

**BAD** example: `rlpEncodedParentNodes`

Fixes OpenST#462
@schemar schemar self-assigned this Dec 6, 2018
deepesh-kn
deepesh-kn reviewed Dec 6, 2018
View reviewed changes
Copy link
Contributor

@deepesh-kn deepesh-kn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

👍
We can consider to change rlpEncodedAccount is used in the following lines.
file: test/data/proof.json Line: 3
file: prove_origin_core.js Line: 36

schemar
schemar commented Dec 7, 2018
View reviewed changes
Copy link
Contributor Author

@schemar schemar left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Eagle eyes again 👀 Updated.

deepesh-kn
deepesh-kn approved these changes Dec 10, 2018
View reviewed changes
Copy link
Contributor

@deepesh-kn deepesh-kn left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM 🏇

@deepesh-kn deepesh-kn merged commit 7e8aa5b into OpenST:develop Dec 10, 2018
@schemar schemar deleted the rlp_names branch December 12, 2018 13:15
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@deepesh-kn deepesh-kn deepesh-kn approved these changes

@0xsarvesh 0xsarvesh Awaiting requested review from 0xsarvesh

@gulshanvasnani gulshanvasnani Awaiting requested review from gulshanvasnani

Assignees

@schemar schemar

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.
3 participants
@schemar @deepesh-kn @benjaminbollen